Expose through extension methods maui build and launch arguments - #19572
Expose through extension methods maui build and launch arguments#19572Frederik St-Onge (frederikstonge) wants to merge 6 commits into
Conversation
@microsoft-github-policy-service agree |
228dae2 to
e17dc45
Compare
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19572Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19572" |
There was a problem hiding this comment.
Pull request overview
Adds public MAUI APIs for customizing build and launch command arguments within the startup pipeline.
Changes:
- Adds synchronous and asynchronous argument callbacks.
- Applies callbacks during MAUI build/launch orchestration.
- Adds documentation and unit tests.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
src/Aspire.Hosting.Maui/Annotations/MauiBuildArgumentsCallbackAnnotation.cs |
Stores argument callbacks and target steps. |
src/Aspire.Hosting.Maui/Lifecycle/MauiBuildQueueEventSubscriber.cs |
Invokes callbacks during startup. |
src/Aspire.Hosting.Maui/MauiBuildArgumentsCallbackContext.cs |
Exposes callback state and mutable arguments. |
src/Aspire.Hosting.Maui/MauiBuildArgumentsExtensions.cs |
Adds public fluent configuration APIs. |
src/Aspire.Hosting.Maui/MauiBuildStep.cs |
Defines build and launch phases. |
src/Aspire.Hosting.Maui/README.md |
Documents argument customization. |
tests/Aspire.Hosting.Maui.Tests/MauiBuildArgumentsExtensionsTests.cs |
Tests API registration and callback behavior. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Suppressed comments (6)
Previously missed (1) — in code that hasn't changed since the last review.
src/Aspire.Hosting.Maui/MauiBuildArgumentsExtensions.cs:112
- This example recommends overriding
NoBuild=true, but that flag is required by the serialized build queue: the semaphore is released when DCP reports the launch process Running, before any launch-time build finishes, allowing the next target to build concurrently. KeepNoBuild=trueand use a non-build-triggering launch property in the example, unless the queue lifecycle is redesigned.
This issue also appears on line 143 of the same file.
/// <example>
/// Force a build during launch by overriding the default <c>-p:NoBuild=true</c>:
/// <code lang="csharp">
/// maui.AddAndroidEmulator("emulator")
/// .WithMauiLaunchArguments(context => context.Arguments.Add("-p:NoBuild=false"));
/// </code>
src/Aspire.Hosting.Maui/Lifecycle/MauiBuildQueueEventSubscriber.cs:293
- This is not idempotent as the new regression test expects. After the first
BeforeStartEvent,launchOverrideis the replacement annotation containing the callback's additions, so a second event copies those edited arguments and applies the callbacks again (and the test helper also registers another subscriber). TheWithMauiLaunchArguments_MultipleStarts_DoNotAccumulateassertion will therefore observe duplicate-p:NoBuild=falseentries. Preserve the pristine override separately or mark the launch callbacks as applied so repeated startup processing cannot reapply them.
var arguments = new List<string>(launchOverride.Arguments);
src/Aspire.Hosting.Maui/README.md:244
- This TypeScript example does not match the generated SDK. A getter-only
IList<string>is exposed as the asynccontext.arguments()method returning anAspireList<string>, whose mutation API isadd;context.arguments.push(...)therefore does not compile. Fetch the list and awaitaddin an async callback.
.withMauiBuildArguments(context => { context.arguments.push("-p:MyProperty=Value"); });
src/Aspire.Hosting.Maui/MauiBuildArgumentsExtensions.cs:42
- These are new exported callback APIs, but the PR only adds C# tests; the existing MAUI polyglot AppHost does not exercise either generated method or its callback context. Add a TypeScript polyglot scenario that mutates the generated arguments editor. This is especially important here because the README currently uses a generated API shape that does not compile.
[AspireExport]
public static IResourceBuilder<T> WithMauiBuildArguments<T>(
this IResourceBuilder<T> builder,
Func<MauiBuildArgumentsCallbackContext, Task> callback)
src/Aspire.Hosting.Maui/README.md:258
- This TypeScript example does not match the generated SDK. A getter-only
IList<string>is exposed as the asynccontext.arguments()method returning anAspireList<string>, whose mutation API isadd;context.arguments.push(...)therefore does not compile. Fetch the list and awaitaddin an async callback.
.withMauiLaunchArguments(context => { context.arguments.push("-p:NoBuild=false"); });
src/Aspire.Hosting.Maui/MauiBuildArgumentsExtensions.cs:148
- This example also recommends overriding
NoBuild=true, which breaks the build queue's serialization because the semaphore can be released as soon as the launch process reports Running, before launch-time compilation completes. KeepNoBuild=trueand demonstrate a property that does not trigger another build.
/// <example>
/// Force a build during launch by overriding the default <c>-p:NoBuild=true</c>:
/// <code lang="csharp">
/// maui.AddAndroidEmulator("emulator")
/// .WithMauiLaunchArguments(context => context.Arguments.Add("-p:NoBuild=false"));
/// </code>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Suppressed comments (4)
Previously missed (2) — in code that hasn't changed since the last review.
src/Aspire.Hosting.Maui/Lifecycle/MauiBuildQueueEventSubscriber.cs:293
- Repeated application is not idempotent: this copies the last override, which already contains the prior callback result. An append-style launch callback therefore adds its argument again whenever
BeforeStartEventis published again, contrary to the documented contract and the new multiple-start test. Preserve the pristine launch override separately and rebuild the callback result from that baseline each time.
var arguments = new List<string>(launchOverride.Arguments);
src/Aspire.Hosting.Maui/MauiBuildArgumentsCallbackContext.cs:51
- This token is not always tied to a resource start. Launch callbacks receive the
BeforeStartEventtoken, while build callbacks receive the linked per-resource token, so the current public documentation promises cancellation semantics that launch callbacks do not have.
/// Gets a token that is cancelled if the resource start is cancelled.
src/Aspire.Hosting.Maui/README.md:258
- This TypeScript sample uses an API shape the generated SDK does not provide.
argumentsis an async getter returning anAspireList<string>, whose mutation method is asynchronousadd; it is not an array withpush.
.withMauiLaunchArguments(context => { context.arguments.push("-p:MyProperty=Value"); });
src/Aspire.Hosting.Maui/MauiBuildArgumentsExtensions.cs:39
- The new exported callback contract has only C# tests that invoke annotations directly. Because the callback context projects a live mutable list into generated SDKs, add ATS/TypeScript generation coverage for both methods and the
arguments(): Promise<AspireList<string>>mutation path; this would also catch the invalid TypeScript examples in this PR.
[AspireExport]
| await PublishBeforeStartAsync(app); | ||
|
|
||
| var launchOverride = Assert.Single(emulator.Resource.Annotations.OfType<ProjectLaunchArgsOverrideAnnotation>()); | ||
| Assert.Equal(["build", "--no-restore", "/t:Run", "-p:NoBuild=true", "-p:NoBuild=false"], launchOverride.Arguments); |
| mauiApp.addAndroidEmulator("emulator") | ||
| .withMauiBuildArguments(context => { context.arguments.push("-p:MyProperty=Value"); }); |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
src/Aspire.Hosting.Maui/MauiBuildArgumentsCallbackContext.cs:51
- This description is inaccurate for launch callbacks: they run from
BeforeStartEventand receive the application-start cancellation token, before any individual resource start can be canceled. Describe this as cancellation of the current callback operation so the documentation is correct for both build and launch steps.
/// <summary>
/// Gets a token that is cancelled if the resource start is cancelled.
/// </summary>
src/Aspire.Hosting.Maui/README.md:264
- This TypeScript fluent call is also left unawaited, so the sample relies on implicit promise flushing and defers failures. Await the registration chain directly.
mauiApp.addAndroidEmulator("emulator")
.withMauiLaunchArguments(async context => {
const args = await context.arguments();
await args.add("-p:MyProperty=Value");
});
| /// Mutate <see cref="Arguments"/> in place to add, remove, or replace the arguments that will be | ||
| /// passed to <c>dotnet</c> for the <see cref="Step"/> this callback is registered for. | ||
| /// </remarks> | ||
| [AspireExport(ExposeProperties = true)] |
| // redact them before logging. Ordinal comparison because these are literal command-line tokens. | ||
| private readonly HashSet<string> _sensitiveArguments = new(StringComparer.Ordinal); | ||
|
|
||
| private readonly IList<string> _arguments; |
| /// build pipeline replaces its value with a placeholder in the arguments it logs to the resource | ||
| /// output. Launch-step arguments are additionally masked by the dashboard's command-line display. | ||
| /// </remarks> | ||
| public void AddArgument(string argument, bool isSensitive = false) |
| /// <summary> | ||
| /// Appends an argument whose value is sensitive (for example <c>-p:AndroidSigningKeyPass=…</c>). | ||
| /// </summary> | ||
| /// <param name="argument">The full argument to add.</param> | ||
| /// <param name="isSensitive"></param> |
| MSBuild properties can carry secrets (for example a signing key password). Add those with `AddSensitiveArgument` instead of `Arguments.Add`. The value is still passed to `dotnet` verbatim, but the build pipeline redacts it from the arguments it writes to the resource logs. Launch-step arguments are additionally masked by the dashboard's command-line display. | ||
|
|
||
| ```csharp | ||
| mauiApp.AddAndroidEmulator() | ||
| .WithMauiBuildArguments(context => context.AddSensitiveArgument($"-p:AndroidSigningKeyPass={keyPassword}")); |
| if (isSensitive) | ||
| { | ||
| _sensitiveArguments.Add(argument); | ||
| } |
Adam Ratzman (adamint)
left a comment
There was a problem hiding this comment.
Thanks for taking this on. I reproduced #19571 against the base branch and reviewed the current head (0bf8a933) end to end.
I don't think this is ready yet:
- A clean rebuild of
Aspire.Hosting.Mauifails with fourCS1574errors because the public docs reference a missingMauiBuildArgumentsCallbackContext.Argumentsmember. - The generated TypeScript surface cannot support the checked-in
context.arguments()/context.addArgument(...)fixture as implemented. isSensitiveonly redacts the MAUI build log. Launch arguments are copied into a string-onlyProjectLaunchArgsOverrideAnnotation, so the README's dashboard-masking guarantee does not hold.- With only warnings-as-errors relaxed, the focused test class runs 18 tests and 2 fail before reaching the launch callback because the builder has no DCP paths.
I left two additional inline comments for the failing lifecycle coverage and launch-argument precedence.
| var tempFile = Path.Combine(workspace.Path, "TempMauiProject.csproj"); | ||
| File.WriteAllText(tempFile, MauiTestHelper.CreateProjectContent("net10.0-android")); | ||
|
|
||
| var appBuilder = DistributedApplication.CreateBuilder(); |
There was a problem hiding this comment.
These two lifecycle tests are currently red. With only TreatWarningsAsErrors disabled to get past the XML-doc errors, the focused class runs 18 tests and these two fail in DcpNameGenerator because DistributedApplication.CreateBuilder() leaves CliPath and DashboardPath unset. Please use TestDistributedApplicationBuilder.Create() (as the nearby MauiPlatformExtensionsTests does), or otherwise configure the required paths, so the assertions actually exercise OnBeforeStartAsync.
| // Swap the immutable annotation for one carrying the updated arguments; DCP reads the last | ||
| // override annotation when it renders the launch command during PrepareProjectExecutables. | ||
| resource.Annotations.Remove(launchOverride); | ||
| resource.Annotations.Add(new ProjectLaunchArgsOverrideAnnotation(arguments, launchOverride.LeadingResourceArgumentToRemove)); |
There was a problem hiding this comment.
This callback edits only ProjectLaunchArgsOverrideAnnotation, not the complete launch argument list. DCP emits that override before resource arguments, while ConfigurePlatformResource adds the MAUI TFM/device properties through WithArgs. A callback that adds -p:AdbTarget=..., -p:_DeviceName=..., or -p:RuntimeIdentifier=... is therefore followed by MAUI's original value and loses under MSBuild's last-value-wins behavior. Please apply the callback to the complete evaluated launch arguments, or otherwise ensure callback edits are emitted last; as written, callers cannot reliably override existing MAUI launch properties.
Description
Allow users to edit maui build and launch arguments to give more flexibility.
Fixes #19571
Checklist
<remarks />and<code />elements on your triple slash comments?